[API-373] Add dashboard field flags to create/update#78
Open
callumreid wants to merge 1 commit into
Open
Conversation
Expose description, is_favorite, is_default, position, and config on `coval dashboards create` and `coval dashboards update`, plus the same fields on the dashboard response (with a DEFAULT table column). Mirrors the v1 API gaining full dashboard field parity.
📝 WalkthroughWalkthroughThis PR extends the dashboard resource with optional metadata fields. The Dashboard model now supports description, is_default, is_favorite, position, and config. CLI commands for create and update accept these new fields as arguments, serialize them into API payloads, and table output includes a DEFAULT column showing which dashboard is the organization default. ChangesDashboard Metadata and Default Status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/dashboards.rs`:
- Around line 65-78: Add input validation to enforce position >= 0: implement a
helper fn validate_position(position: Option<i64>) -> Result<()> that returns an
error (e.g., bail!("--position must be >= 0")) when a Some value is negative,
and call validate_position(...) from the Create and Update command handlers
before constructing the request (similar to validate_widget_grid used at lines
~192-204) so negative --position values are rejected client-side with a clear
CLI error.
- Around line 88-101: The position field allows negative values despite the doc
comment; update the dashboard update argument validation to enforce position >=
0 by adding a validator for the position field (the same approach used in
CreateArgs) — either add a clap value parser/range (e.g. value_parser with range
0..) or a small validate() check in the UpdateArgs handling path that returns an
error when position < 0; target the position field in the dashboards.rs
UpdateArgs/struct and mirror the CreateArgs validation logic so negatives are
rejected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 58efad1a-2da8-4700-ad81-1103d18cfb44
📒 Files selected for processing (4)
README.mdsrc/client/models/dashboard.rssrc/commands/dashboards.rstests/cli_tests.rs
Comment on lines
+65
to
+78
| #[arg(long)] | ||
| description: Option<String>, | ||
| /// Mark as a favorite (true/false). | ||
| #[arg(long)] | ||
| favorite: Option<bool>, | ||
| /// Make this the organization's default dashboard (true/false). Omit to auto-default the first dashboard. | ||
| #[arg(long)] | ||
| default: Option<bool>, | ||
| /// Ordering position (>= 0). Omit to append to the end. | ||
| #[arg(long)] | ||
| position: Option<i64>, | ||
| /// Free-form JSON config: a JSON string or @path to a file. | ||
| #[arg(long)] | ||
| config: Option<String>, |
There was a problem hiding this comment.
Add validation for position >= 0.
The doc comment at line 73 states position should be ">= 0", but there's no validation to enforce this constraint before sending the request. Users providing negative values will receive an API error instead of a clear, immediate CLI error.
Consider adding a validation helper similar to validate_widget_grid (lines 192-204):
fn validate_position(position: Option<i64>) -> Result<()> {
if let Some(p) = position {
if p < 0 {
bail!("--position must be >= 0");
}
}
Ok(())
}Then call it in the Create and Update handlers before building the request.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/dashboards.rs` around lines 65 - 78, Add input validation to
enforce position >= 0: implement a helper fn validate_position(position:
Option<i64>) -> Result<()> that returns an error (e.g., bail!("--position must
be >= 0")) when a Some value is negative, and call validate_position(...) from
the Create and Update command handlers before constructing the request (similar
to validate_widget_grid used at lines ~192-204) so negative --position values
are rejected client-side with a clear CLI error.
Comment on lines
+88
to
+101
| #[arg(long)] | ||
| description: Option<String>, | ||
| /// Mark as a favorite (true/false). | ||
| #[arg(long)] | ||
| favorite: Option<bool>, | ||
| /// Make this the organization's default dashboard (true/false). Setting true unsets any other default. | ||
| #[arg(long)] | ||
| default: Option<bool>, | ||
| /// Ordering position (>= 0). | ||
| #[arg(long)] | ||
| position: Option<i64>, | ||
| /// Replacement free-form JSON config: a JSON string or @path to a file. | ||
| #[arg(long)] | ||
| config: Option<String>, |
There was a problem hiding this comment.
Add validation for position >= 0.
Same issue as CreateArgs: the doc comment at line 96 specifies ">= 0" but no validation is present.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/dashboards.rs` around lines 88 - 101, The position field allows
negative values despite the doc comment; update the dashboard update argument
validation to enforce position >= 0 by adding a validator for the position field
(the same approach used in CreateArgs) — either add a clap value parser/range
(e.g. value_parser with range 0..) or a small validate() check in the UpdateArgs
handling path that returns an error when position < 0; target the position field
in the dashboards.rs UpdateArgs/struct and mirror the CreateArgs validation
logic so negatives are rejected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
coval dashboards create/updateonly accepted--name. This adds the rest of the dashboard fields the v1 API exposes (paired backend PR coval-ai/backend#4498) so the CLI can fully manage dashboards — including marking one as the organization's default/dashboard.Linear: API-373
New flags (create + update)
--description--favoritetrue/false--defaulttrue/false--position>= 0)--config@fileThese compose with
--input-jsonlike the existing flags. The dashboard response model and--format jsonoutput now includedescription,is_default,is_favorite,position, andconfig, and the table view gains aDEFAULTcolumn.Tests
cargo fmt,cargo clippy --all-targets -D warnings, andcargo testall pass. Added two tests asserting the new flags are serialized into the create POST and update PATCH bodies (viabody_partial_json) and that responses carrying the new fields parse.No version bump here — per this repo's convention version bumps land in a separate
chore:PR after merge.Summary by CodeRabbit
New Features
Documentation
Tests
Proof
CI: all 5 platform builds (macOS arm64/x64, Linux arm64/x64, Windows x64) +
checkgreen on this PR. Reproduced locally:fmt + clippy —
cargo fmt --checkclean;cargo clippy --all-targets -- -D warningsclean (no warnings).coval dashboards create --help(new flags wired, booleans constrained totrue|false):Tests —
cargo test→74 passed; 0 failed. The two new tests assert the flags serialize into the request body (wiremockbody_partial_json) and that responses carrying the new fields parse:test_dashboard_create_with_full_fieldsrunsdashboards create --name Ops --description desc --favorite true --default true --position 3 --config '{"date_preferences":{"preset":"last-7-days"}}'and asserts the POST body containsdisplay_name/description/is_favorite/is_default/position/config— a wrong flag→JSON-key mapping (e.g.--defaultnot mapping tois_default) would 404 the mock and fail the test.Note: a live run against current prod isn't shown because the new fields are server-side-gated on coval-ai/backend#4498 — current prod silently ignores unknown request fields, so a prod run wouldn't actually exercise them until that PR is deployed. End-to-end proof against the real API lands via that PR's post-merge integration suite.